Skip to content

feat(query): add opt-in distributed broadcast join planner - #9404

Open
discord9 wants to merge 45 commits into
GreptimeTeam:mainfrom
discord9:feat/dist-join-planner
Open

discord9 wants to merge 45 commits into
GreptimeTeam:mainfrom
discord9:feat/dist-join-planner

Conversation

@discord9

@discord9 discord9 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

Uses the nested datanode scan capability from #9206, now merged. This branch is synchronized with upstream main 28ec3ff5aad8d274d83ae42f96dc54f838fb6b20 by ordinary merge; the earlier synchronization point was b913be2af77a5a936bcccb3d5b50358d1313a777.

What's changed and what's your intention?

Add an opt-in, default-off experimental_dist_join path for supported two-scan INNER equijoins. Broadcast the smaller input to the larger table's datanodes instead of joining both remote scans on the frontend.

  • Put SMALL on DataFusion's left/hash-build side and BIG on the local right/probe side; restore SQL column order with the existing Projection. BIG-region routing/pruning and remote dynamic-filter producer identity remain intact. DataFusion's JoinSelection remains in control; there is no forced global input order.
  • Supply ordinary MergeScan row estimates from selected unique regions' leader heartbeat observations with checked arithmetic. Rows are Inexact; memory and column statistics remain unknown. Disk size is a placement heuristic, not a hash-memory estimate. Unknown or unusable statistics leave planning unchanged.
  • Verify complete results, schema, execution roles and pruning through the actual SQL setting. No shuffle/general distributed join, self-join or aggregate-side support is added. No new service, protobuf schema or persisted format is introduced.

Limits: Query admission is unchanged. Positive max_concurrent_queries can cause nested requests to time out while outer streams hold permits; default-disabled tests do not establish finite-quota success. Typed read preference is preserved across supporting receivers; old receivers ignore its internal carrier. This change does not introduce enterprise bootstrap or automatic follower lifecycle. Earlier release/ECS measurements used an older BIG-build implementation and are not performance evidence for this SMALL-build implementation. A fresh release/ECS run on this implementation is reported below.

Earlier verification on parent 1f22baf446d7ac94f1178dcdad6a4a85dad8e191 (before the latest main synchronization):

  • cargo check --locked -p query -p operator -p session -p meta-client -p datanode -p flow -p servers -p tests-integration --tests — passed.
  • cargo nextest run --locked -p query -p operator -p session -p common-session -p meta-client --lib --retries 0 -j 6 --no-fail-fast — 1,118 passed, 2 existing skips, no retries.
  • cargo nextest run --locked -p tests-integration --lib --retries 0 -j 1 --no-fail-fast -E 'test(nested_merge_scan_capability_test) | test(test_flow_requester_health_check_and_handle_request)' — 6 passed, including real SQL planning/execution and nested whole-stream/strict-ingress failure checks.
  • Datanode installed-factory regression — 1 passed.
  • cargo nextest run --locked -p cmd --features dev-tools --retries 0 -j 4 --no-fail-fast — 150 passed, 0 skipped, no retries.
  • cargo nextest run --locked -p servers -p datanode --lib --retries 0 -j 2 --no-fail-fast -E 'test(grpc::greptime_handler::tests) | test(http::hints::tests) | test(http::read_preference::tests) | test(test_region_server_parallelism)' — 8 selected tests passed, no retries; 690 other tests were excluded by the filter, not run as a full suite.
  • cargo clippy --locked -p cmd --features dev-tools --all-targets -- -D warnings — passed.
  • cargo build --locked -p cmd --features dev-tools --bin greptime --bin query_perf_fixture --bin query_regression_runner -p sqlness-runner --bin sqlness-runner — passed. The freshly built server reports this exact commit and clean: true; all four binary hashes were recorded and verified.
  • Prebuilt sqlness-runner bare -t '^(standalone|distributed):(create|view|dist_join)$' -c tests/cases --bins-dir "$CARGO_TARGET_DIR/debug" — 7/7 passed: common create, view/create and view/view in both environments, plus distributed explain/dist_join. All runner-owned cluster processes stopped; git diff --exit-code -- tests/cases confirms generated expectations did not change. Final workspace formatting and diff checks passed.
  • cargo clippy --locked -p query -p operator -p session -p meta-client -p datanode -p flow -p servers -p substrait@1.4.0-alpha.0 -p tests-integration --all-targets -- -D warnings — passed.
  • Workspace formatting, TOML, license-header/enterprise-license, spelling, Snafu and non-generated diff checks — passed. Generated SQL expectations are retained from the feature parent, not hand-edited.

The helper/protocol/parallelism regressions, four freshly built binaries and all seven real SQLness cases passed on the earlier clean parent 1f22baf446d7ac94f1178dcdad6a4a85dad8e191 (details above), not on the latest re-synchronized head. Draft-skipped jobs are not functional test passes. The checked-in SQL test retains off→on→off plans and all five ordered six-column rows. The executed real SQL integration checks all 4,877 projected rows against frontend and known references, full schema, actual datanode SMALL hash-build/BIG probe metrics, and BIG-region pruning.

Enterprise companion validation before the latest public re-sync: The previously synchronized public source and enterprise companion were revalidated in an isolated checkout. This combination has not been revalidated against the latest public head. All 10 selected nested/wire/Region/Flow tests passed without retries, using the normal enterprise pre-build hook and DatanodeBuilder. Real outer/inner TCP Flight strict-Follower reads returned the complete known/frontend result; absence of followers failed the complete stream with no nested Leader fallback. Four supported typed-preference protobuf roundtrips, strict ingress/inner-failure checks, and the actual SQL broadcast JOIN's full rows/schema/build-probe roles/pruning passed. The strict Flow caller regression and six-related-crate all-target lint, formatting and locked checks also passed. Production source is unchanged by the validation-only test overlay; the formal enterprise companion pins upstream main containing merged #9206, not an unmerged feature or a private test commit. A freshly built formal enterprise binary also passed a separate real metasrv/datanode/frontend CLI run: normal pre-build hook, exact complete three-column/three-row SQL, matching clean build identity in all three startup logs, and successful cleanup of all three owned processes. Physical followers are opened explicitly by the focused fixture; the ordinary CLI check and the focused TCP Follower check are separate evidence. Automatic follower lifecycle and a single CLI Follower end-to-end scenario are not claimed.

Latest main-sync status on 38f91de408d: The branch accepts upstream #9491's removal of MergeScan hash-partitioning advertisement (try_with_new_distribution and partition_cols plumbing), retaining the logical broadcast JOIN rewrite, row estimates, producer identities, captured filters and per-region metrics. The eight-crate locked check and selected library/nested JOIN/Flow test chain completed successfully after the merge; the earlier library-test total above has not been independently re-counted for this head. Strict affected-crate Clippy completed before the formatting check failed; formatting was then corrected and workspace formatting plus strict query all-target Clippy passed. Independent read-only follow-up review approved the final source and confirmed closure of the constructor-arity, unused-binding and producer-ID coverage findings.

Latest-head runtime verification: Latest head 47f3b2998b1 passed the full non-Draft CI (Integration CI run 38349416691 and Rust CI run 38349416607, both completed/success): all five Sqlness suites, all four Fuzz Chaos jobs, all unit/integration tests green. This head includes the P1 volatile-expression guard and P2 wire-flag propagation fixes from review. The older paragraphs below still record earlier-parent evidence: the post-merge seven-case invocation used the previously built server, so it is not validation of those heads. Helper/protocol suite counts, fresh-binary evidence and the performance result below remain evidence for the earlier parent only. Generated expectations have not been changed by this re-sync; existing generated-result EOF blank-line warnings from range diff checks are not described as a clean full-range diff check.

Release/ECS performance measured on earlier parent 1f22baf446d7ac94f1178dcdad6a4a85dad8e191: The existing query-regression workflow was dispatched once with immutable base b913be2af77a5a936bcccb3d5b50358d1313a777 (upstream main) and candidate 1f22baf446d7ac94f1178dcdad6a4a85dad8e191 on a dedicated ecs.u1-c1m1.2xlarge runner (qreg-ecs-38042793447), release profile, case tests/perf/query_cases/sql_join_dist_broadcast/case.toml. The run succeeded. The case query enables experimental_dist_join and runs the fact/dimension broadcast join with aggregation and top-10 ordering. Results: base median 193.17 ms / p95 207.42 ms; candidate median 54.00 ms / p95 55.78 ms (-72.0%, threshold max_candidate_latency_regression_pct passed, 0 validation errors). All three timed samples per side returned the identical eight ordered host/zone/score rows (maximum absolute score difference 1.4e-12); the executed_plan_preflight check returned status 200. This is the current SMALL-hash-build implementation's own measurement, not the earlier BIG-build numbers.

PR Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.
  • This PR needs to be backported to release branches, and I have added the backport-<target> labels (e.g. backport-v1.3 targets release/v1.3).

@github-actions github-actions Bot added docs-required This change requires docs update. size/XXL labels Sep 29, 2026
Give a datanode the ability to decode and execute a plan whose MergeScan
carries a nested plan (e.g. an inner MergeScan over a build table), so a
distributed sub-plan pushed to a datanode can itself fan out to other
datanodes. This is the execution/codec capability only; it is not wired
to any user-facing switch and produces no plan by itself.

Codec (src/query/src/query_engine/default_serializer.rs):
- MergeScanAwareSerializer keeps the request's SessionState and the
  engine catalog manager, and decodes a MergeScan payload's tables by
  (catalog, schema, table) name through the engine catalog rather than
  the region-aware request catalog list (which binds every table to the
  request's region and breaks on mismatched columns).

Datanode (src/datanode):
- New DatanodeRegionQueryHandler (region_query.rs) serves region queries
  for the MergeScan nodes of a received plan, resolving region leaders
  via PartitionRuleManager and dispatching to peer datanodes.
- Wire KvBackendCatalogManager + PartitionRuleManager + NodeClients into
  the datanode's catalog/cache registry (with_dist_planner stays false:
  a datanode never rewrites its own plan).

Bug fixes uncovered while wiring this up:
- cache registry: register the table/table_info/table_name/table_route/
  partition_info caches the datanode's catalog manager and partition
  rule manager require (previously missing -> panic on lookup).
- default_serializer: decode_sub_plan bridge no longer drives a future
  on a current-thread runtime from within another executor (spawn a
  dedicated thread + runtime when not on a multi-thread runtime).

Tested by query_engine::default_serializer unit tests: nested MergeScan
round-trip and engine-catalog resolution with mismatched column names.
No user-facing behavior change.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
… state

SessionStateBuilder::new_from_existing collects the function registry
(HashMap) into a Vec and build() re-registers each entry; register_udf
registers aliases before the function's own name and the Vec order is
unstable. Rebuilding the session state to install MergeScanAwareSerializer
after the GreptimeDB functions were registered could therefore rebind a
name/alias (e.g. date_format, an alias of DataFusion's to_char) back to
the built-in function. This path runs for every decoded plan, not only
nested MergeScan ones.

Extract the GreptimeDB function registration into register_greptime_functions
and re-apply it after the final build of both the top-level decode state and
the MergeScan payload state. Add regression tests asserting date_format stays
bound to the GreptimeDB implementation on both paths.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Three follow-ups from review of the nested MergeScan execution capability:

1. Layer the datanode caches (P2b). table_cache derives from
   table_info_cache + table_name_cache, and partition_info_cache derives
   from table_route_cache, but they were all registered in a single flat
   CacheRegistry whose invalidate() runs concurrently with no ordering.
   A derived cache could be invalidated and refilled from a base cache
   that had not been invalidated yet, leaving stale metadata. Build the
   base caches and the derived caches as separate layers of a
   LayeredCacheRegistry (reusing the existing layering mechanism) so base
   caches finish invalidating before derived ones. Both the production
   path (cmd) and the test scaffolding now build the same layered
   registry; the flat build_datanode_cache_registry is removed.

2. Don't let a nested MergeScan's internal region query compete for the
   concurrency permit (P2a). An outer query holds its permit until its
   stream is fully consumed, and a nested MergeScan makes the datanode
   issue region queries to peer datanodes as execution stages of that
   same query. If such an inner stage also took a permit, the outer query
   waiting on it would deadlock against its own held permit (or time out
   when max_concurrent_queries > 0). Mark inner DN->DN region queries as
   internal stages: QueryRequest gains an  flag carried over
   the wire in the request header's query_context extensions (reserved
   key, not injectable via hints), and the region server skips acquiring
   a concurrency permit for internal-stage requests on both acquire
   sites. Outer queries still acquire normally.

3. Add an end-to-end capability test
   (tests-integration/.../nested_merge_scan_capability_test.rs): hand-build
   Join(<local probe region>, MergeScan(build regions on both datanodes))
   with mismatched column names and duplicate build keys, ship it to a
   datanode over a real TCP Flight endpoint, consume the stream fully and
   compare the row multiset against the frontend's reference join result,
   assert a real cross-datanode DoGet via an RPC counter, and assert an
   inner region failure fails the whole query. Also migrate the
   with_real_datanode_grpc_addr + DatanodeRpcStats test scaffolding.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
…on fork

ScalarUDFImpl's trait bounds changed so its impl can no longer be
downcast via as_any(); assert the function equals the GreptimeDB
implementation instead, which to_char can never satisfy.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Add an end-to-end regression test for the nested MergeScan capability
under max_concurrent_queries = 1: both datanodes' only permit is held by
the outer queries' streams while their inner MergeScans fetch the build
regions from the same datanodes over real Flight connections. The result
consumption is bounded by a timeout, so a broken link anywhere in the
chain (marking the inner request internal, carrying the marker in the
request header, honoring it at the receiving acquire sites) fails the
test instead of deadlocking.

Also add a minimal with_datanode_options_override hook to the cluster
builder, and fix the test module doc: the outer request is handed to
RegionServer::handle_remote_read in process; only the inner region
queries go over the wire.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
The table route cache uses InitStrategy::VersionChecked, whose version
counter belongs to the whole CacheContainer rather than to a single
table: any TableId invalidation bumps the shared version, so a cold
load of one table's route retries when an unrelated table is
invalidated concurrently. Document this as a known limitation on the
region query handler, and pin the behavior with an integration test:
run the nested plans while a background task invalidates an unrelated
table id on every datanode, assert the results still match the expected
multiset (retries only add work, they never return a stale route), and
report the observed load amplification via the cache miss counter
(2 loads without invalidations, 8 over three runs with 24 unrelated
invalidations in this run). Also expose the datanode cache registries
on the test cluster so a test can drive the local invalidation path
that the heartbeat handler drives after a metasrv broadcast.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
…join

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
…ss support

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
@discord9
discord9 force-pushed the feat/dist-join-planner branch from e9763f6 to 694489f Compare October 4, 2026 09:50
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
…tures

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
@discord9
discord9 marked this pull request as ready for review October 12, 2026 03:24
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
The default query path must not receive row statistics that could flip
DataFusion JoinSelection from Partitioned to CollectLeft. Only compute
row estimates when the session explicitly opts in with SET
experimental_dist_join = true.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
dist_join.result: the default (non-opt-in) physical plan now stays a
partitioned hash join (RepartitionExec on both sides) after row
estimates are only computed when experimental_dist_join is enabled;
drop the CollectLeft-shaped expectation recorded while estimates were
injected unconditionally.

explain.result: DistJoinPlanner is a registered analyzer rule, so
EXPLAIN VERBOSE now prints its 'logical_plan after DistJoinPlanner'
stage (same pattern as DistPlannerAnalyzer/JsonSchemaConcretizeRule).

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
…t join

Reviewer findings on the opt-in nested broadcast join:

P1: the rewritten plan re-executes the SMALL-side subplan on every
BIG-side region, so volatile expressions (e.g. random()) on the SMALL
side change result semantics within one query. Add a volatility guard:
contains_volatile_expr inspects the stripped small-side plan
(projections, filters, table-scan filters, recursively) and refuses
the rewrite. Conservative fallback only; a shared-materialization
implementation is a separate stack PR.

P2: experimental_dist_join never crossed the QueryContext protobuf
roundtrip, so the datanode's nested MergeScan planning always skipped
row estimates and JoinSelection could flip to BIG-build when only the
append-only BIG side had exact statistics. Follow the read_preference
extension-key precedent: encode query.experimental_dist_join in the
protobuf extensions when enabled, restore on decode.

Tests: volatile-small-side rejection unit test, protobuf roundtrip
unit tests, and a real-cluster regression test asserting every
dispatched BIG region's HashJoinExec builds the 2-row SMALL side and
probes the append-only BIG side.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
The P1 guard rejected volatile expressions inside the small-side
sub-plan but missed volatile expressions that appear only in
join.on keys. ExtractEquijoinPredicate can extract volatile
expressions into join keys (e.g. `ON b.k = CASE WHEN random() < 0.5
THEN s.k ELSE s.k + 1 END`), and the physical-plan-time Projection
that computes such keys would be replicated per BIG region after
the broadcast rewrite.

Extend the guard to also check small-side join key expressions for
volatility, and add a unit test covering this shape.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Declare the volatile test UDF as non-nullable via
return_field_from_args so FilterNullJoinKeys does not inject a
volatile IS-NOT-NULL filter into the small sub-plan. Add assertions
that verify the test premise: the small plan has no volatile
expressions and the volatile expression exists only in the JOIN key,
ensuring the test exercises the join-key guard specifically.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>

@fengjiachun fengjiachun left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two inline findings from static review of this head. No local builds or tests were run.

.collect(),
join.filter.clone(),
join.join_type,
join.join_constraint,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that the rewritten payload is serializable before moving the join into MergeScan. An INNER JOIN ... USING (k) has non-empty equijoin keys and can pass these guards when the table statistics favor broadcasting, but this preserves JoinConstraint::Using. The pinned DataFusion Substrait producer explicitly rejects that constraint (producer/rel/join.rs:31–34). RegionRequester::handle_query then propagates the encoding error, so enabling the optimization makes a previously executable query such as SELECT b.k FROM big b INNER JOIN small s USING (k) fail.

The earlier DistPlannerAnalyzer encoding check does not cover the new remote payload created here. Could we retain the original plan if the rewritten payload cannot be encoded, and add an off/on SQL execution regression for USING? Explicitly excluding Using would address this case, while checking the whole payload would also cover other unsupported expressions.

)
.ok()?;
let restored_plan = restore_join_schema(swapped_join, join.schema.clone())?;
let mut merge_scan = MergeScanLogicalPlan::new(restored_plan, false);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The admission-limit caveat in the PR description can make a single selected query unable to complete. With max_concurrent_queries = 1, if a datanode owns both an outer probe region and an inner build region, the outer request acquires the only permit in RegionServer::handle_remote_read_inner and retains it through PermitGuardedStream. Executing this nested plan then sends another read to the same datanode; that read waits for the permit, while the outer stream needs its result before it can finish and release the permit. The query therefore fails when the limiter wait times out. Larger limits have the same problem when outer streams occupy all permits.

Could we prevent this rewrite when nested reads cannot make progress under admission control, or provide bounded parent/child admission that avoids the wait cycle? Please add a same-node regression with a positive limit (especially 1), plus a saturated multi-node case. The existing tests use the default disabled limiter and do not cover this configuration.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-required This change requires docs update. size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants